Skip to content

Fix critical custody, LP, lottery, mint, and FFI regressions#35

Draft
cursor[bot] wants to merge 5 commits into
mainfrom
cursor/critical-bug-investigation-5e56
Draft

Fix critical custody, LP, lottery, mint, and FFI regressions#35
cursor[bot] wants to merge 5 commits into
mainfrom
cursor/critical-bug-investigation-5e56

Conversation

@cursor

@cursor cursor Bot commented Jul 1, 2026

Copy link
Copy Markdown

Issue

  • Issue: daily critical bug-finding automation
  • Branch: cursor/critical-bug-investigation-5e56
  • Scope: Fix concrete high-severity regressions found in recent commits: unauthorized token minting, lottery draw-block ticket purchases, unauthorized LP position exits, incorrect Uniswap V4 ERC20 settlement, unauthenticated Gravity custody payouts, and default-enabled Foundry FFI.
  • Out of scope: Broad refactors and intentional Gravity denom-mapping exploit PoC behavior.
  • Issue analysis: N/A - cron investigation, not a GitHub issue workflow.

Summary

  • Restricts critical asset-moving entry points to the intended controller/owner/submitter before funds or LP NFTs can be moved.
  • Keeps Foundry FFI disabled by default and requires RUN_FFI_TESTS=true plus --ffi for host-command-backed tests.
  • Restores CI/pre-commit validation flow, including explicit FFI test lanes and a Manticore-compatible build profile.

Agent Handoffs

  • Issue analysis file: N/A
  • Handoff file: N/A
  • Verification file: N/A
  • Architecture digest: N/A - automation bug-fix PR, no issue artifact required.

Files To Read First

  1. src/25-gravity/Gravity.sol
  2. src/21-uniswap-v3/UniswapV3LiquidityNftExample.sol
  3. src/22-uniswap-v4/UniswapV4PositionManagerExample.sol
  4. src/22-uniswap-v4/UniswapV4PoolManagerExample.sol
  5. foundry.toml, .github/workflows/test.yml, scripts/pre-commit-ci.sh
  6. Regression tests under test/14-stake-together, test/15-simple-lottery, test/21-uniswap-v3, test/22-uniswap-v4, test/25-gravity, and FFI tests.

Issue Conflict Check

  • Cron investigation found concrete critical bugs with plausible triggers.
  • Conflicts required user confirmation.

Design / Diagram Updated

  • No architecture-impacting issue artifact required for this automation PR.
  • Updated docs/issues/<issue-number>/architecture.md

Validation Evidence

  • forge fmt --check
  • Targeted tests:
    • forge test --match-path test/14-stake-together/StakeTogether.t.sol
    • forge test --match-path test/15-simple-lottery/SimpleLottery.t.sol
    • forge test --match-path test/21-uniswap-v3/UniswapV3LiquidityNftExample.t.sol
    • forge test --match-path test/22-uniswap-v4/UniswapV4WrapperUnit.t.sol
    • forge test --match-path test/22-uniswap-v4/UniswapV4PositionManagerExample.t.sol
    • forge test --match-path test/25-gravity/GravityBridgeExploit.t.sol
    • default-mode FFI skip checks for test/FFI.t.sol, test/DifferentialTest.t.sol, test/Vyper.t.sol
  • Explicit FFI opt-in tests:
    • RUN_FFI_TESTS=true forge test --match-path test/FFI.t.sol --ffi -vv
    • RUN_FFI_TESTS=true forge test --match-path test/DifferentialTest.t.sol --ffi -vv
    • RUN_FFI_TESTS=true forge test --match-path test/Vyper.t.sol --ffi -vv with ~/.local/bin on PATH after pip install -r requirements.txt
  • forge test - 498 passed, 0 failed, 36 skipped
  • FOUNDRY_PROFILE=manticore forge build --build-info --sizes

Security Findings

  • Critical: CloudCoin.mint was unrestricted, allowing arbitrary reward-token inflation. Fixed with Ownable and onlyOwner; regression test added.
  • Critical: SimpleLottery.purchaseTicket accepted tickets at the draw block if timestamp had not reached the purchase deadline, changing winner entropy after the draw block was selected. Fixed by closing purchases when block.number >= drawBlock; regression test added.
  • Critical: Uniswap V3/V4 wrapper methods allowed non-owners/non-controllers to decrease, collect, or burn positions when wrapper approvals/custody existed. Fixed by checking position owner/controller before exits; regression tests added.
  • Critical: V4 PoolManager ERC20 settlement transferred tokens before settle() but omitted sync(currency), causing incorrect settlement accounting. Fixed by calling sync(currency) before transfer/settle; unit assertions added.
  • Critical: Gravity submitWithdrawalBatch released custody assets for any caller. Fixed by requiring the constructor deployer batch submitter, while preserving the intended denom-poisoning PoC through authorized batch submission; regression test added.
  • Critical: foundry.toml enabled FFI in default/CI profiles, so ordinary forge test could execute host commands from tests. Fixed by removing profile-level FFI and gating FFI tests behind explicit env+flag opt-in.

User Test Request

  • Review and run the listed validation commands; no manual UI testing is required.
Open in Web View Automation 

cursoragent and others added 5 commits July 1, 2026 11:06
Co-authored-by: lzhkai <lzhkai1111@qq.com>
Co-authored-by: lzhkai <lzhkai1111@qq.com>
Co-authored-by: lzhkai <lzhkai1111@qq.com>
Co-authored-by: lzhkai <lzhkai1111@qq.com>
Co-authored-by: lzhkai <lzhkai1111@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant